-
Notifications
You must be signed in to change notification settings - Fork 42
Add SwiftBuild dlls to cli installer component alongside SwiftPM #417
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
e4d3a59
to
ab10d2d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is technically correct and nothing to be done here.
However, because no good deed goes unpunished - looking at this entry makes me wonder if we are building Swift Build properly. Should all these dynamic libraries be dynamic? Would we do better in terms of size and performance to convert some of them to static? I do not have the answer to that, and that is something that requires experimentation. That is something that we should really consider and evaluate (and make the appropriate changes here).
That work however IMO should be a follow up change. Packaging and distributing SwiftBuild as a starting point is the right thing to do.
One final question: should this also go into 6.2?
Right now building all the libraries as dynamic simplifies the build by making it easy to ensure we're picking the right linker driver to pull in the c++ stdlib + blocks runtime in the right targets. With some tweaks to swift-driver and general build config cleanup, I think we can gain a lot of flexibility here and experiment with alternate layouts. That said, we've been shipping the fully dynamically linked version on macOS for quite awhile without issues, so I'm not too concerned about perf impact.
I intend to cherrypick to 6.2 but I'd like to make sure this + the SwiftPM changes are working end-to-end on main first |
Please do a cross-repo test with building the toolchain for Windows and Windows ARM64 before merging. |
hmm seeing failures in the cross-repo toolchain build
Which is true, TOOLCHAIN_ROOT isn't on the command line:
|
ab10d2d
to
2ddf127
Compare
Think I just needed a rebase, my local checkout was old |
2ddf127
to
dca65b9
Compare
TOOLCHAIN_ROOT -> ToolchainRoot to align with the changes I rebased on |
Verified that the toolchains built successfully and can be installed: I can launch SwiftPM without any missing DLL errors, which is a good sign that nothing is broken at runtime. I couldn't test much more than that because swiftc.exe is crashing in mimalloc when compiling manifests:
I expect this is probably unrelated to the changes here since the compiler/driver should be unaffected, but I'll see if there's anything I can double-check - any concerns on your end @compnerd? |
The invalid memory access is somewhat concerning. There could be something with symbolic resolution that could be going wrong. I think that we should get a backtrace on the invalid memory access and go from there. |
It looks like the driver is crashing while exec-ing the frontend in mimalloc's atexit handler. This is happening so early in the execution of the driver that heap corruption seems unlikely, but so does a mimalloc bug.
|
Wait, we might be mixing arm and x86 here where we shouldn't be... |
Just a heads up, if you're saying that because of the "AMD64" and "ARM64EC" in the backtrace, that's expected. All system libraries in Windows 11 are special "ARM64EC" binaries which can be loaded into both arm64 and x86_64 processes. It's kind of like our Universal binaries. Very different, but conceptually similar. |
Yeah, ARM64EC seems wrong ... |
The AMD64 is on our binaries - we are mixing up architectures. |
But why is that an issue? From what I understood, arm64ec and amd64 are supposed to mix, and a Windows 11 on ARM system contains only arm64ec system dlls. |
I think this might actually be a CI config issue somewhere, in https://ci-external.swift.org/job/swift-PR-build-toolchain-windows-arm64/23/consoleFull it looks like the job which is supposed to build arm64 toolchains is using x86_64-unknown-windows-msvc everywhere |
This won't cross-compile, you need to do CC: @shahmishal |
Let me see if I can do some workarounds in build-windows-toolchain.bat |
This is my first time touching the windows installer, so there may be some silly mistakes here. Opening the PR now so I can run some tests